Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Escape braces for runner path #3408

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Escape braces for runner path #3408

wants to merge 1 commit into from

Conversation

RushingAlien
Copy link
Contributor

Description

Hello, as you all know braces () are commonly used in filenames, steam's Proton 9 is still named Proton 9.0 (Beta). The braces caused an sh syntax error as per attached issue. This PR escapes the braces for runner path.

Fixes #3407

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.

  • Use a runner with braces in the name
  • run a windows program
  • run legacy wine tools
    • winecfg
    • explorer
    • uninstaller
    • debugger
    • control panel
    • taskmgr

@github-actions GitHub Actions
Copy link
Contributor

Pylint result on modfied files: 
************* Module bottles.backend.wine.winecommand
bottles/backend/wine/winecommand.py:90:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
bottles/backend/wine/winecommand.py:90:4: R0913: Too many arguments (11/5) (too-many-arguments)
bottles/backend/wine/winecommand.py:135:12: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:141:12: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:397:20: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:401:51: E1136: Value 'gpu['prime']['integrated']' is unsubscriptable (unsubscriptable-object)
bottles/backend/wine/winecommand.py:403:20: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:182:8: W0612: Unused variable 'is_nvidia' (unused-variable)
bottles/backend/wine/winecommand.py:449:12: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:458:12: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:476:4: R0913: Too many arguments (6/5) (too-many-arguments)
bottles/backend/wine/winecommand.py:511:32: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
bottles/backend/wine/winecommand.py:520:21: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
bottles/backend/wine/winecommand.py:529:21: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
bottles/backend/wine/winecommand.py:545:20: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:550:20: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:555:20: W0105: String statement has no effect (pointless-string-statement)
bottles/backend/wine/winecommand.py:648:8: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
bottles/backend/wine/winecommand.py:659:8: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
bottles/backend/wine/winecommand.py:697:12: R1705: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it (no-else-return)
bottles/backend/wine/winecommand.py:718:23: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

@JakobDev
Copy link
Contributor

JakobDev commented Jul 2, 2024

Why is manual escaping even needed here? 'm not familiar with the codebase, but the subprocess modules (which should be used) of Python does escaping automatic.

@RushingAlien
Copy link
Contributor Author

RushingAlien commented Jul 2, 2024 •
edited
Loading

See linked issue, and I do not know if the subprocess module is used or not, only that the sh command that bottles executes to start a program in wine fails in a syntax error if there's braces in path.

and the change I've made are additions to an existing manual escape for spaces. I'm just adding one for braces.

@jntesteves
Copy link
Contributor

Why is manual escaping even needed here? 'm not familiar with the codebase, but the subprocess modules (which should be used) of Python does escaping automatic.

Quoting is used everywhere in the codebase because subprocess.Popen is being called with shell=True. From a quick look, I'm not sure if that shell=True is really necessary, but removing it would require rewriting a lot of stuff, so manual quoting it is for now. Manually quoting should be just as safe, as long as we never forget to do it, like what happened here.

@jntesteves
Copy link
Contributor

@RushingAlien if you're not confortable with doing the change I suggested and changing the commit message to reflect that, I'll open a new PR to resolve this issue.

@jntesteves jntesteves mentioned this pull request Jul 3, 2024
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Escape more special characters
3 participants
@RushingAlien @JakobDev @jntesteves